Skip to content

use claude to remove some warnings#11044

Open
Jorropo wants to merge 11 commits into
meshtastic:developfrom
Jorropo:fix/warnings
Open

use claude to remove some warnings#11044
Jorropo wants to merge 11 commits into
meshtastic:developfrom
Jorropo:fix/warnings

Conversation

@Jorropo

@Jorropo Jorropo commented Jul 17, 2026

Copy link
Copy Markdown
Member

🤝 Attestations

  • I have tested that my proposed changes behave as described.
  • I have tested that my proposed changes do not cause any obvious regressions on the following devices:
    • Heltec (Lora32) V3
    • LilyGo T-Deck
    • LilyGo T-Beam
    • RAK WisBlock 4631
    • Seeed Studio T-1000E tracker card
    • Other (please specify below)

Summary by CodeRabbit

  • Bug Fixes
    • Improved display of device short names across identity, startup, and OEM screens.
    • Corrected canned-message delivery banners to show “unknown” when a node name isn’t available.
    • Improved Bluetooth connection parameter handling for high-throughput and lower-power modes.
    • Preserved correct rendering across E-Ink, OLED, and TFT display variants.
  • Refactor
    • Clarified fallthrough intent and tightened local variable handling/annotation in scanning and graphics rendering paths without changing visible behavior.

@Jorropo Jorropo changed the title Fix/warnings use claude to remove some warnings Jul 17, 2026
@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 6d8362ec-560b-47b6-8120-e9fa8299ca96

📥 Commits

Reviewing files that changed from the base of the PR and between 659baee and 5e13e8e.

📒 Files selected for processing (6)
  • src/gps/GPS.cpp
  • src/graphics/SharedUIDisplay.cpp
  • src/graphics/draw/DebugRenderer.cpp
  • src/graphics/draw/NotificationRenderer.cpp
  • src/graphics/draw/UIRenderer.cpp
  • src/modules/TrafficManagementModule.cpp
🚧 Files skipped from review as they are similar to previous changes (4)
  • src/graphics/draw/DebugRenderer.cpp
  • src/modules/TrafficManagementModule.cpp
  • src/graphics/draw/UIRenderer.cpp
  • src/graphics/draw/NotificationRenderer.cpp

📝 Walkthrough

Walkthrough

The pull request adds compiler annotations, updates UI character-array checks and conditional calculations, removes unused maintenance locals, and changes NimBLE connection-parameter handling from updates to requests.

Changes

Firmware cleanup and behavior adjustments

Layer / File(s) Summary
Compile-time annotations and local cleanup
src/detect/ScanI2CTwoWire.cpp, src/gps/GPS.cpp, src/graphics/SharedUIDisplay.cpp, src/graphics/draw/DebugRenderer.cpp, src/mesh/aes-ccm.cpp
Intentional fallthroughs and potentially unused graphics locals are explicitly annotated; constant-time cast formatting is adjusted.
UI rendering and display conditions
src/graphics/draw/NotificationRenderer.cpp, src/graphics/draw/UIRenderer.cpp, src/modules/CannedMessageModule.cpp
Rendering checks use character-array contents, E-ink-only calculations are scoped conditionally, and TFT region calculations are simplified.
Maintenance local removal
src/modules/TrafficManagementModule.cpp
Unused clock and configuration locals are removed while cache maintenance continues unchanged.
BLE connection parameter requests
src/nimble/NimbleBluetooth.cpp
Connection setup paths call requestConnParams(...) instead of updateConnParams(...).

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested labels: enhancement

Suggested reviewers: ndoo, jp-bennett

🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description only includes the attestation checklist and omits the required PR summary and testing details. Add a brief PR summary plus completed testing notes and device/regression coverage, and indicate any untested hardware.
Docstring Coverage ⚠️ Warning Docstring coverage is 43.75% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title is related to the change, but it is too vague to convey the actual warning cleanup work. Use a more specific title that names the main cleanup, such as the affected modules or compiler-warning fixes.
✅ Passed checks (2 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

⚡ Try this PR in the Web Flasher

Note

Building this pull request… the flash button, badges and supported-board
list will appear here automatically once CI finishes.

*/
LOG_INFO("BLE requestHighThroughputConnection");
bleServer->updateConnParams(conn_handle, 6, 12, 0, 600);
bleServer->requestConnParams(conn_handle, 6, 12, 0, 600);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we know what the behavioral differences are between this and the old methods?

@Xaositek

Copy link
Copy Markdown
Contributor

Tried this before under #10698 - ended up closing it based upon comments.

@Xaositek Xaositek added enhancement New feature or request bugfix Pull request that fixes bugs 2.8 cleanup Code cleanup or refactor needs-review Needs human review ai-generated Possible AI-generated low-quality content labels Jul 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.8 ai-generated Possible AI-generated low-quality content bugfix Pull request that fixes bugs cleanup Code cleanup or refactor enhancement New feature or request needs-review Needs human review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants